Fix potential segmentation faults with exif. (#639)

1. A pointer exif_app_ to an ExifApp on QList<ExifApp> exif_apps was saved.
After this the list was modified.  This can lead to the saved pointer becoming
invalid.
2. The ExifApp structure had a dtor, but no other special functions, i.e.  it
violated the rule of 3 and the rule of 5.  Operations on QList<ExifApp> may
cause an ExifApp on the list to be copied or destroyed.  If an ExifApp is
destroyed, then the ExifApp dtor would close the files, even though there could
be a copy of the ExifApp expecting the files still to be open.

This scenerio occured with Qt6, causing segmentation faults in exif.test.

diff --git a/exif.cc b/exif.cc
index d2f71a7f516046c265a182ad57846b0a81e42c27..783406fd849380a2369b5891250f35fc5fcedccd 100644 (file)
--- a/exif.cc
+++ b/exif.cc
@@ -177,20 +177,10 @@ struct ExifApp {
   gbfile* fcache{nullptr};
   gbfile* fexif{nullptr};
   QList<ExifIfd> ifds;
-
-  ~ExifApp()
-  {
-    if (fcache) {
-      gbfclose(fcache);
-    }
-    if (fexif) {
-      gbfclose(fexif);
-    }
-  }
 };
 
 static gbfile* fin_, *fout_;
-static QList<ExifApp>* exif_apps;
+static QList<ExifApp*>* exif_apps;
 static ExifApp* exif_app_;
 static const Waypoint* exif_wpt_ref;
 static QDateTime exif_time_ref;
@@ -345,6 +335,15 @@ exif_read_datestamp(const ExifTag* tag)
 static void
 exif_release_apps()
 {
+  for (auto* app : qAsConst(*exif_apps)) {
+    if (app->fcache) {
+      gbfclose(app->fcache);
+    }
+    if (app->fexif) {
+      gbfclose(app->fexif);
+    }
+    delete app;
+  }
   delete exif_apps;
   exif_apps = nullptr;
 }
@@ -375,8 +374,8 @@ exif_load_apps()
   exif_app_ = nullptr;
 
   while (! gbfeof(fin_)) {
-    exif_apps->append(ExifApp());
-    ExifApp* app = &exif_apps->last();
+    exif_apps->append(new ExifApp);
+    ExifApp* app = exif_apps->last();
     app->fcache = gbfopen(nullptr, "wb", MYNAME);
 
     app->marker = gbfgetuint16(fin_);
@@ -1360,8 +1359,7 @@ exif_write_apps()
 {
   gbfputuint16(0xFFD8, fout_);
 
-  for (auto& app_instance : *exif_apps) {
-    ExifApp* app = &app_instance;
+  for (auto* app : qAsConst(*exif_apps)) {
 
     gbfputuint16(app->marker, fout_);
 
@@ -1476,7 +1474,7 @@ static void
 exif_rd_init(const QString& fname)
 {
   fin_ = gbfopen_be(fname, "rb", MYNAME);
-  exif_apps = new QList<ExifApp>;
+  exif_apps = new QList<ExifApp*>;
 }
 
 static void
@@ -1508,7 +1506,7 @@ exif_wr_init(const QString& fname)
   exif_success = 0;
   exif_fout_name = fname;
 
-  exif_apps = new QList<ExifApp>;
+  exif_apps = new QList<ExifApp*>;
 
   fin_ = gbfopen_be(fname, "rb", MYNAME);
   is_fatal(fin_->is_pipe, MYNAME ": Sorry, this format cannot be used with pipes!");